From: Bartosz Dziewoński <matma.rex@gmail.com>
Date: Fri, 17 Jun 2016 15:20:11 +0000 (+0200)
Subject: Introduce new hook UploadVerifyUpload to allow preventing file uploads
X-Git-Tag: 1.31.0-rc.0~6403^2~1
X-Git-Url: http://git.cyclocoop.org/%7D%7Cconcat%7B?a=commitdiff_plain;h=8f2acfcde440b6476a8139d72765b6dda6b46842;p=lhc%2Fweb%2Fwiklou.git

Introduce new hook UploadVerifyUpload to allow preventing file uploads

The new hook runs at the beginning of UploadBase::performUpload().
Unlike the existing UploadVerifyFile hook, the new one provides the
information about the file description page being created, and the
user who is performing the upload. It also does not run for uploads
to stash.

The action=upload API and Special:Upload have been changed to treat
errors from UploadBase::performUpload() as recoverable, which means
that they will attempt to stash the file (previously they would require
the user to upload it again). This is mostly intended for errors from
the new hook, but I think it makes sense for the existing ones, which
are mostly transient filesystem erorrs.

Bug: T89302
Change-Id: Ie68801b307de8456e1753ba54a29c34c8063bc36
---

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 1d118939cd..0e7c1c0f68 100644
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -3285,6 +3285,19 @@ $mime: (string) The uploaded file's MIME type, as detected by MediaWiki.
   representing the problem with the file, where the first element is the message
   key and the remaining elements are used as parameters to the message.
 
+'UploadVerifyUpload': Upload verification, based on both file properties like
+MIME type (same as UploadVerifyFile) and the information entered by the user
+(upload comment, file page contents etc.).
+$upload: (object) An instance of UploadBase, with all info about the upload
+$user: (object) An instance of User, the user uploading this file
+$props: (array) File properties, as returned by FSFile::getPropsFromPath()
+$comment: (string) Upload log comment (also used as edit summary)
+$pageText: (string) File description page text (only used for new uploads)
+&$error: output: If the file upload should be prevented, set this to the reason
+  in the form of array( messagename, param1, param2, ... ) or a MessageSpecifier
+  instance (you might want to use ApiMessage to provide machine-readable details
+  for the API).
+
 'UserIsBot': when determining whether a user is a bot account
 $user: the user
 &$isBot: whether this is user a bot or not (boolean)
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 0a79aa496f..3af0dff015 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -350,9 +350,10 @@ class ApiUpload extends ApiBase {
 	 * @param array $error Error array suitable for passing to dieUsageMsg()
 	 * @param string $parameter Parameter that needs revising
 	 * @param array $data Optional extra data to pass to the user
+	 * @param string $code Error code to use if the error is unknown
 	 * @throws UsageException
 	 */
-	private function dieRecoverableError( $error, $parameter, $data = [] ) {
+	private function dieRecoverableError( $error, $parameter, $data = [], $code = 'unknownerror' ) {
 		try {
 			$data['filekey'] = $this->performStash();
 			$data['sessionkey'] = $data['filekey'];
@@ -365,6 +366,9 @@ class ApiUpload extends ApiBase {
 		if ( isset( $parsed['data'] ) ) {
 			$data = array_merge( $data, $parsed['data'] );
 		}
+		if ( $parsed['code'] === 'unknownerror' ) {
+			$parsed['code'] = $code;
+		}
 
 		$this->dieUsage( $parsed['info'], $parsed['code'], 0, $data );
 	}
@@ -754,9 +758,14 @@ class ApiUpload extends ApiBase {
 				$this->mParams['text'], $watch, $this->getUser(), $this->mParams['tags'] );
 
 			if ( !$status->isGood() ) {
-				$error = $status->getErrorsArray();
-				ApiResult::setIndexedTagName( $error, 'error' );
-				$this->dieUsage( 'An internal error occurred', 'internal-error', 0, $error );
+				// Is there really no better way to do this?
+				$errors = $status->getErrorsByType( 'error' );
+				$msg = array_merge( [ $errors[0]['message'] ], $errors[0]['params'] );
+				$data = $status->getErrorsArray();
+				ApiResult::setIndexedTagName( $data, 'error' );
+				// For backwards-compatibility, we use the 'internal-error' fallback key and merge $data
+				// into the root of the response (rather than something sane like [ 'details' => $data ]).
+				$this->dieRecoverableError( $msg, null, $data, 'internal-error' );
 			}
 			$result['result'] = 'Success';
 		}
diff --git a/includes/specials/SpecialUpload.php b/includes/specials/SpecialUpload.php
index 4b731cb994..0ef6af161f 100644
--- a/includes/specials/SpecialUpload.php
+++ b/includes/specials/SpecialUpload.php
@@ -535,7 +535,7 @@ class SpecialUpload extends SpecialPage {
 		);
 
 		if ( !$status->isGood() ) {
-			$this->showUploadError( $this->getOutput()->parse( $status->getWikiText() ) );
+			$this->showRecoverableUploadError( $this->getOutput()->parse( $status->getWikiText() ) );
 
 			return;
 		}
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index ba5171f523..5d0bc13d82 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -717,13 +717,23 @@ abstract class UploadBase {
 	 */
 	public function performUpload( $comment, $pageText, $watch, $user, $tags = [] ) {
 		$this->getLocalFile()->load( File::READ_LATEST );
+		$props = $this->mFileProps;
+
+		$error = null;
+		Hooks::run( 'UploadVerifyUpload', [ $this, $user, $props, $comment, $pageText, &$error ] );
+		if ( $error ) {
+			if ( !is_array( $error ) ) {
+				$error = [ $error ];
+			}
+			return call_user_func_array( 'Status::newFatal', $error );
+		}
 
 		$status = $this->getLocalFile()->upload(
 			$this->mTempPath,
 			$comment,
 			$pageText,
 			File::DELETE_SOURCE,
-			$this->mFileProps,
+			$props,
 			false,
 			$user,
 			$tags